-
Notifications
You must be signed in to change notification settings - Fork 197
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[draft] Remove option mathjax_classes #406
[draft] Remove option mathjax_classes #406
Conversation
The global approach in 395 isn't really sustainable: it requires all-ways cooperation between all projects that want to customize MathJax. Additionally, when processing a MyST document without Sphinx, the MathJax configuration changes are not performed (part of executablebooks#347). And, of course, this approach of overriding the MathJax object causes issues down the line for projects that need to customize MathJax (the setting in Sphinx isn't sufficient, see sphinx-doc/sphinx#9450) The following two approaches would not cause these issues: 1. Add a custom script instead of touching the mathjax3_config variable; something like this, essentially: ```js app.add_js_file(None, priority=0, body=""" var MathJax = window.MathJax || MathJax; MathJax.options = MathJax.options || {}; MathJax.options.processHtmlClass = (MathJax.options.processHtmlClass || "") + "|math"; """) ``` - Don't touch MathJax_config at all; instead, add an explicit `mathjax_process` class on all math nodes, either by changing `docutils_renderer` (this PR) or by adding a Docutils transform to processes all math nodes: ```python class ActivateMathJaxTransform(Transform): default_priority = 800 @staticmethod def is_math(node): return isinstance(node, (math, math_block)) def apply(self, **kwargs): for node in self.document.traverse(self.is_math): node.attributes.setdefault("classes", []).append("mathjax_process") ``` This PR isn't ready for merging; it's just to start a discussion.
Thanks for submitting your first pull request! You are awesome! 🤗 |
Yeh but this would break any other extension that adds math and likely has not added explicitly the |
I don't understand — isn't it the reverse? What MyST currently does breaks extensions that use math. |
No because the configuration sets mathjax to run on all elements with the |
Unless, of course, some other extensions modifies the Mathjax config (which is the issue at hand); and unless the document is processed with Docutils instead of Sphinx, too. That's why I was suggesting to add mathjax_process on relevant nodes. But it's probably OK to leave this unaddressed. |
The global approach in #395 isn't really sustainable: it requires all-ways cooperation between all projects that want to customize MathJax. Additionally, when processing a MyST document without Sphinx, the MathJax configuration changes are not performed (part of #347). And, of course, this approach of overriding the MathJax object causes issues down the line for projects that need to customize MathJax (the setting in Sphinx isn't sufficient, see sphinx-doc/sphinx#9450)
@chrisjsewell you wrote:
I think the following two approaches would not require overriding the global config:
Add a custom script instead of touching the mathjax3_config variable; something like this, essentially:
Don't touch MathJax_config at all; instead, add an explicit
mathjax_process
class on all math nodes, either by changingdocutils_renderer
(this PR) or by adding a Docutils transform to processes all math nodes:This PR isn't ready for merging; it's just to start a discussion.